Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc_resolve: allow use crate_name; under uniform_paths. #54005

Merged
merged 1 commit into from
Sep 7, 2018

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Sep 6, 2018

Specifically, use crate_name; and use crate_name::{self, ...}; are now allowed, whereas previously there would produce a (false positive) ambiguity error, as the ambiguity detection code was seeing the crate_name import as a locally-available definition to conflict with the crate.

r? @petrochenkov cc @aturon @joshtriplett @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 6, 2018
@joshtriplett
Copy link
Member

LGTM, awesome!

@aturon
Copy link
Member

aturon commented Sep 6, 2018

Setting priority to land for beta ASAP

@bors p=100

@Centril
Copy link
Contributor

Centril commented Sep 6, 2018

This is great 🎉💯❤️

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 6, 2018

📌 Commit 31fce91 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2018
@bors
Copy link
Contributor

bors commented Sep 6, 2018

⌛ Testing commit 31fce91 with merge ad7b22e...

bors added a commit that referenced this pull request Sep 6, 2018
rustc_resolve: allow `use crate_name;` under `uniform_paths`.

Specifically, `use crate_name;` and `use crate_name::{self, ...};` are now allowed, whereas previously there would produce a (false positive) ambiguity error, as the ambiguity detection code was seeing the `crate_name` import as a locally-available definition to conflict with the crate.

r? @petrochenkov cc @aturon @joshtriplett @Centril
@bors
Copy link
Contributor

bors commented Sep 7, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing ad7b22e to master...

@bors bors merged commit 31fce91 into rust-lang:master Sep 7, 2018
@eddyb eddyb deleted the uniform-paths-self-resolve branch September 7, 2018 08:38
bors added a commit that referenced this pull request Sep 10, 2018
rustc_resolve: inject `uniform_paths` canaries regardless of the feature-gate, on Rust 2018.

This PR is an attempt at future-proofing "anchored paths" by emitting the same ambiguity errors that `#![feature(uniform_paths)]` would, with slightly changed phrasing (see added UI tests).

Also, on top of #54005, this PR allows this as well:
```rust
use crate_name;
use crate_name::foo;
```
In that any ambiguity between an extern crate and an import *of that same crate* is ignored.

r? @petrochenkov cc @aturon @Centril @joshtriplett
bors added a commit that referenced this pull request Sep 14, 2018
rustc_resolve: don't treat uniform_paths canaries as ambiguities unless they resolve to distinct Def's.

In particular, this allows this pattern that @cramertj mentioned in #53130 (comment):
```rust
use log::{debug, log};
fn main() {
    use log::{debug, log};
    debug!(...);
}
```
The canaries for the inner `use log::...;`, *in the macro namespace*, see the `log` macro imported at the module scope, and the (same) `log` macro, imported in the block scope inside `main`.

Previously, these two possible (macro namspace) `log` resolutions would be considered ambiguous (from a forwards-compat standpoint, where we might make imports aware of block scopes).

With this PR, such a case is allowed *if and only if* all the possible resolutions refer to the same definition (more specifically, because the *same* `log` macro is being imported twice).
This condition subsumes previous (weaker) checks like #54005 and the second commit of #54011.

Only the last commit is the main change, the other two are cleanups.

r? @petrochenkov cc @Centril @joshtriplett
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants